This is still broken b/w the 600px and 782px breakpoints.

Noting a related comment I found in a recent PR at #11904 (comment).

I'd rather Gutenberg added margins by default, and theme authors manually overrode that on the front end if for some reason they'd prefer to.

That would:

• Ensure that by default, text-based columns have the margins they need.
• Bring greater consistency between the front and back end.

@designsimply Thanks for linking that PR! I had previously tried to find discussion about how the columns were even supposed to look but hadn't seen that.

The best way to show what's going on with the margins is to use a CSS debugging utility like Pesticide. The screenshot below are three rows of column blocks. A 2 column, a 3 column, and a 4 column.

As you can see, all rows have 0px margin on the left but the 3 column row is given a margin-right. This is because the row ends with an odd(:nth-child(odd)) numbered column(3).
5, 7, 9... columns would do the same thing.

The logic used here for the column margins doesn't add up.
As I mentioned above

the :not(:last-child) used on the column would only "not add" the margin. It didn't remove it if the column was odd numbered.

The method used for the text-column CSS works correctly:

	.wp-block-column {
		margin: 0 16px;
		padding: 0;

		&:first-child {
			margin-left: 0;
		}

		&:last-child {
			margin-right: 0;
		}
	}

Maybe we should go with something like that.
Or even better, only have one .wp-block-column style that covers both block types. SCSS folder structure be damned. 😀

The other separate problem noticed and mentioned by @drdogbot7 above, is that the design isn't fluid between breakpoints since our columns CSS is splitting one major layout change between two breakpoints, break-medium and break-small. The components rely on each other breaking at the same time. We need to decide on one, medium or small bp.

@jasmussen does this make sense?

This solution works, but I think you're eliminating one of the original breakpoints.

Based on the comments, this is the original intent:

Mobile: less than 600px
all columns full width and stacked. No margins.

Small: 600px - 782px
columns wrap in rows of 2. Margins on the inside.

Medium: bigger than 782px
all columns fit in one row. Margins on the inside.

My solution #12408, attempts to keep the Small breakpoint and make it work.

Honestly… I'd be fine with simplifying things and losing the 2-column breakpoint. I just want it fixed!

Thanks, all, for the work here. This needs a thorough testing and I will test as soon as I can. In the mean time, this just a 👍 👍 for helping uncover and fix these issues.

Honestly… I'd be fine with simplifying things and losing the 2-column breakpoint.

I think that's fine too if it solves a problem or simplifies things.

Thanks again for working on this. Took it for a good spin, and it seems to be working as intended. Nice work. It also doesn't seem to break a few of the flex things that were difficult to get working, notably around making sure content doesn't grow larger than its parent contents:

As I look at this on admittedly extreme 8-column setups, I'm finding myself missing the 2-column midway breakpoint as noted in #12199 (comment). But if there's a good reason for why we can't restore that, that's okay too, just want to make sure we do this with a will.

Expanding the testing range to Kjell who was instrumental in helping to ship this.

Some more testing, on twentyeleven and twentynineteen, with forced backgrounds to clearly verify. Interestingly, the twentynineteen theme "cheated" by bundling their own fix.

twentyeleven

Before	After

twentynineteen

Before	After

(The extended height of the last column in twentynineteen may be an unintended consequence of this style rule?)

This seems to fix things on the front end:

... but I'm still seeing additional margins applied on the back end:

Can anyone else confirm that?

Like @jasmussen, I do miss the 2-columns at middle-breakpoints a bit. It'd be cool to include that if possible.

(The extended height of the last column in twentynineteen may be an unintended consequence of this style rule?)

I think you're right. I can get that fixed in the theme. 👍 On second thought... I can't find any instances where this actually causes any problems, and visually these end up being the same height. So I'll let it be for now.

As I look at this on admittedly extreme 8-column setups, I'm finding myself missing the 2-column midway breakpoint as noted in #12199 (comment). But if there's a good reason for why we can't restore that, that's okay too, just want to make sure we do this with a will.

At 600px, a most 3+ column layouts are gonna look squished. If we're going to lose the 2-column breakpoint, then I think I'd prefer to keep the columns stacked until 782px.

That would also make it easier for theme developers to add a 600px-782px breakpoint if they want to, because they wouldn't have to override as many styles.

Submitted a new pull request (#12816) that fixes the margins and keeps the 2-column breakpoint. Added fixes to the editor stylesheet as well.

I can't remember if I noticed the extra margin in the editor or not. I don't see where this style was duplicated or any styles at all for .wp-block-column in editor.css so I would think the change in style.css would take affect in the editor. I'll test it out in a few.

I think a breakpoint at 2 columns would be nice too. Does it currently have one. If I removed it, I didn't intend to.
I consolidated a couple redundant @include break-small() for simplicity sake. Would this have removed it?

I can't remember if I noticed the extra margin in the editor or not. I don't see where this style was duplicated or any styles at all for .wp-block-column in editor.css so I would think the change in style.css would take affect in the editor. I'll test it out in a few.

The issue is that there's a lot of extra markup in the editor view, so the editor.scss needs to be adjusted to account for that. In the editor the .wp-block-column styles are applied to:

.wp-block-columns > .editor-inner-blocks > .editor-block-list__layout > [data-type="core/column"]

I think a breakpoint at 2 columns would be nice too. Does it currently have one. If I removed it, I didn't intend to.
I consolidated a couple redundant @include break-small() for simplicity sake. Would this have removed it?

I think it was supposed to have a 2-column breakpoint, but it didn't really work. Styles were added at the 'small' breakpoint that should have been split between 'small' and 'medium'.

From what I can tell, the margins that were added with ODD and EVEN rules were supposed to be for the 'small' (2-column) breakpoint where the columns wrap, and the not:(:first/last-child) rules were intended to apply at the 'medium' breakpoint where the columns all fit in one row.

At the 'medium' breakpoint, the ODD/EVEN rules also need to be reset or you end up with that "unintentional right margin".

I just updated the editor.css to match style.css for the margins if @kjellr or anyone wants to test. Seems to have done the trick for me in the editor.

One other thing I wanted to clarify about intended design for "columns".

The styles in this PR more or less duplicate the styles for Text Column block except that the text column has a 16px right/left margin while this column has 32px right/left margin.
Text column:

.wp-block-text-columns .wp-block-column {
	margin: 0 16px;
}

vs the general column block:

.wp-block-column {
	margin-left: 32px;
	margin-right: 32px;
}

Realize with the columns sitting side by side, our column block here actually has 64px of separation between columns while the text-column has 32px between. Is this the original intent?

I'm wondering if like @aduth was thinking here #12199 (comment) the intent of the even/odd was to not double up margins in between. And 32px is actually the desired space between columns?

I just updated the editor.css to match style.css for the margins if @kjellr or anyone wants to test. Seems to have done the trick for me in the editor.

Yes, that latest update works on my end. 👍

Thanks to @drdogbot7 I finally think I understand the overall design intent of the current CSS.
I opened this PR only thinking in terms of the extra right margin on desktop.

I'll break down my understanding and the code to accomplish it.

• Each row of columns should be flush with the container, as are other blocks.

• On mobile, a single column should take up the full width of the screen.

• A little larger, @include break-small. Two columns, separated by a space, should take up the full width.

The column width should be half the screen(50%) - half our margin since we have 2 cols but only one margin

flex-basis: calc(50% - #{$grid-size-large});

We only need the margin before the second column in the row (even)

:nth-child(even) {
	margin-left: $grid-size-large * 2;
}

• On medium sized screens and up, @include break-medium. All columns are in a single row, separated by a space but still flush with the container at the beginning and the end of the row.

We add the margin before all columns except for the first in the row. Right margins are never add so we don't worry about the last column

:not(:first-child) {
	margin-left: $grid-size-large * 2;
}

I'm not crazy about the specificity added by pseudo-classes but I don't think it's avoidable and it's not too bad here.

I've updated this PR and tested.

fixing a mistake or just making an opinionated change?

Both kinda. I'm guessing that's how it was intended to look before all the iterations took it elsewhere.
Just a guess though. I could be wrong

Exactly right @drdogbot7! I just updated to account for the negative margins added by the editor.
I think you're correct on the flex stuff too but I'm not gonna mess with style.scss anymore. As long as it works we can optimise the CSS choices on down the road.
We need to get this thing merged 😃

Exactly right @drdogbot7! I just updated to account for the negative margins added by the editor.
I think you're correct on the flex stuff too but I'm not gonna mess with style.scss anymore. As long as it works we can optimise the CSS choices on down the road.
We need to get this thing merged 😃

Does it mean it's ready to merge? Can someone give it ✅ after the recent changes applied?

This is a MASSIVE improvement. Thank you so much @m-e-h and @drdogbot7 for your valiant work.

I agree let's get this in ASAP and then push additional fixes separately.

However I'm seeing a small regression in Chrome, and it's directly related to this conversation here #12199 (comment) — GIFs:

This is master, this shows the bug that this PR tries to fix:

Note the additional margin on odd numberd (3, 5) columns.

The following is this branch as it is right now:

A vast improvement, but notice how on EVEN numbered columns (2, 4, 6) the last child has an additional tiny padding.

What's up?

When the code looks like this, the column width is accurate ONLY for odd numbers of columns:

			// Beyond mobile, allow 2 columns.
			@include break-small() {
				flex-basis: calc(50% - (#{$grid-size-large * 2} + #{$block-padding}));
				flex-grow: 0;
			}

When the code looks like this, everything appears to work for me, AND the initial issue appears fixed:

			// Beyond mobile, allow 2 columns.
			@include break-small() {
				flex-basis: 50%;
				flex-grow: 0;
			}

Here's how that looks:

The issue ALSO appears to be fixed if we add the suggested change by @drdogbot7:

			// Beyond mobile, allow 2 columns.
			@include break-small() {
				flex-basis: calc(50% - #{$grid-size-large} - #{$block-padding * 2});
				flex-grow: 0;
			}

Here's how that looks:

I just tested and both flex-basis: 50%; and flex-basis: calc(50% - #{$grid-size-large} - #{$block-padding * 2}); both fix the issue in IE11 and Edge, same as the other browsers. Here's Edge:

Final step:

We need figure out why both flex-basis: 50%; and flex-basis: calc(50% - #{$grid-size-large} - #{$block-padding * 2}); fix the issue, and then we need to pick which one to ship. And THEN we should merge. And THEN we should party.

flex-basis: calc(50% - #{$grid-size-large} - #{$block-padding * 2}); is correct. This resolves to 50% - 44px.

calc(50% - (#{$grid-size-large * 2} + #{$block-padding})); resolves to 50% - 46px, which is close but not quite right. Pretty sure I came up with this so… oops!

flex-basis:50%; blows up the small, 2-column breakpoint, and we're back to where we started. This makes no sense.

flex-basis: calc(50% - #{$grid-size-large} - #{$block-padding * 2}); works for me at both breakpoints. THOUGH we do still have a vertical spacing issue when the columns wrap.

Remember, at the medium breakpoint the columns will shrink to fit the row, but they won't grow any wider than their initial size. When we set flex-basis, we're essentially setting the maximum width of a column.

One unintended consequence of this is that if you have a single column, it won't use all the available space:

I've spent the last few hours fussing with this, and I really think that the only reasonable thing for us to do is to remove the small (2-column) breakpoint entirely from the columns block.

• It is broken in the currently released version, so we're not removing anything that users are relying on.
• It is an opinionated addition that would be better left to themers
• Even with this fix, it is broken Twenty-Nineteen, which will be many users first (possibly last?) experience with this block.

Let's remove it now when it will be least disruptive. This will save us and users a lot of headache moving forward. I think this would be the right course of action even if it means delaying the fix.

EDIT: I created a PR #13605, that does this… just as an example.

I updated the flex-basis calculation. The math is lost on me but it appears to be the values we need. 🤔

I do see the issue with twentynineteen. It's setting the flex-wrap: nowrap; to start at >600px. Looks like they just went with what they were given to work with and the editor columns don't currently have a working medium breakpoint.

As much as I prefer having the two column bp, perhaps @drdogbot7 is correct. While it seems we're fixing a broken implementation of the medium breakpoint, we're actually introducing a new medium breakpoint since there never actually was one. A breaking change? Maybe.

While it seems we're fixing a broken implementation of the medium breakpoint, we're actually introducing a new medium breakpoint since there never actually was one.

To be fair, there were distinct small and medium breakpoints, but the small breakpoint was totally broken, and the medium breakpoint was buggy. In the code, the breakpoints got a bit muddled together which added to the confusion.

At this point some important actively developed themes twenty-nineteen, atomic-theme have implemented their own fixes (as I did for a site I built last month). Even if we restrict ourselves to doing the bare minimum to just squash the bugs (see #12816), we'll probably inadvertently break certain themes… but what's the alternative?

One unintended consequence of this is that if you have a single column, it won't use all the available space:

I would almost suggest an argument can be made this is intentional. In any case it can be fixed separately and shouldn't block this.

Even with this fix, it is broken Twenty-Nineteen, which will be many users first (possibly last?) experience with this block.

I do see the issue with twentynineteen. It's setting the flex-wrap: nowrap; to start at >600px. Looks like they just went with what they were given to work with and the editor columns don't currently have a working medium breakpoint.

Yep. My bad. But we can trivially fix TwentyNineteen with an update, so that's not a problem.

However if you both agree that the medium breakpoint is not productive for this block, I strongly agree it's a good time to remove it. In light of new responsive controls being worked on, perhaps it's even moreso the time.

@m-e-h can you make this final change, and then let's ship this thing?

@m-e-h can you make this final change, and then let's ship this thing?

If we want to ship it in Gutenberg 5.0 it needs to be in the shippable state by tomorrow.

I've been playing with different variations WAY too much tonight. I say ship it like this, with the two column breakpoint.
Removing the two column bp leaves us 1 column still on an 800px screen width. Something just doesn't seem right about that, being that this is a "column" block.

Twentyninetine will need some tweaks but I think that'll be the case even without the 2 column bp.

When testing an unstyled Underscores theme, the 2 column breakpoint version is the only thing that looks great without a total re-write of this CSS.

A complete rewrite will come but I'm thinking the best thing to do now is to fix what's "broken", which is the right margin on medium+ screens and the jagged layout small+ screens.

#13363 is looking like the proper way to handle column styles. Not that it matters too much but I also think having the "mobile","tablet","desktop" breakpoints in place already seems to be in line with the direction of that discussion so far.

Thank you all for the thoughtful feedback,

I'm for merging this now as a vast improvement over what's in master today. You all have good points, and let's continue to explore enhancements in #13363 and even through methods such as #13605. Inaction is only going to make matters worse, and this PR fixes problems.

Here are some GIFs:

It is unfortunate that themes have had to fix issues present, and it will be unfortunate if this bugfix causes further issues. However:

a) this will be not make it to WordPress 5.1 and will likely land in 5.2, so there will be some runway both to become aware of any breakage, and perhaps to even land further improvements

b) themes thankfully have a relatively seamless method of providing updates

I would recommend we merge this in the current state, for all of the discussion provided above.